Skip to content

Conversation

RunDevelopment
Copy link
Contributor

@RunDevelopment RunDevelopment commented Aug 14, 2025

fixes parts of #9613, related to #12962

Unlike #12962, I added support for size_of, align_of, and size_of_val to ConstEval, so all lints using it benefit from this change.

cast_possible_truncation itself changed relatively little. Functions calls are now delegated to const eval and ptr-sized to fixed-sized casts now consider from_nbits.

changelog: [cast_possible_truncation]: support size_of, align_of, and size_of_val


My intention with this PR is not to superceed #12962, but to get in a simple improvement while I work on #15342.

@rustbot
Copy link
Collaborator

rustbot commented Aug 14, 2025

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 14, 2025
Copy link

github-actions bot commented Aug 14, 2025

Lintcheck changes for c5a67a8

Lint Added Removed Changed
clippy::arithmetic_side_effects 0 46 5
clippy::assertions_on_constants 1 0 0
clippy::cast_possible_truncation 0 13 2
clippy::eq_op 7 0 0
clippy::identity_op 3 0 0
clippy::if_same_then_else 1 0 0

This comment will be updated if you push new changes

@rustbot
Copy link
Collaborator

rustbot commented Sep 4, 2025

☔ The latest upstream changes (possibly e9b7045) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great contribution! Sorry for the late review, I've been very busy this past two weeks.

View changes since this review

Comment on lines +518 to +522
ExprKind::Call(callee, [_]) => match self.get_fn_diagnostic_name(callee) {
// `align_of_val` doesn't have a diagnostic name, unfortunately.
// Some(sym::mem_align_of_val) => self.align_of_call(callee),
Some(sym::mem_size_of_val) => self.size_of_call(callee),
_ => None,
Copy link
Member

@blyxyas blyxyas Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the other comment

Suggested change
ExprKind::Call(callee, [_]) => match self.get_fn_diagnostic_name(callee) {
// `align_of_val` doesn't have a diagnostic name, unfortunately.
// Some(sym::mem_align_of_val) => self.align_of_call(callee),
Some(sym::mem_size_of_val) => self.size_of_call(callee),
_ => None,

Comment on lines +508 to +517
ExprKind::Call(callee, []) => match self.get_fn_diagnostic_name(callee) {
Some(sym::i8_legacy_fn_max_value) => Some(Constant::Int(i8::MAX as u128)),
Some(sym::i16_legacy_fn_max_value) => Some(Constant::Int(i16::MAX as u128)),
Some(sym::i32_legacy_fn_max_value) => Some(Constant::Int(i32::MAX as u128)),
Some(sym::i64_legacy_fn_max_value) => Some(Constant::Int(i64::MAX as u128)),
Some(sym::i128_legacy_fn_max_value) => Some(Constant::Int(i128::MAX as u128)),
Some(sym::mem_align_of) => self.align_of_call(callee),
Some(sym::mem_size_of) => self.size_of_call(callee),
_ => None,
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[..] The rest pattern also allows for zero-sized arrays, no need for the extra branch.

Suggested change
ExprKind::Call(callee, []) => match self.get_fn_diagnostic_name(callee) {
Some(sym::i8_legacy_fn_max_value) => Some(Constant::Int(i8::MAX as u128)),
Some(sym::i16_legacy_fn_max_value) => Some(Constant::Int(i16::MAX as u128)),
Some(sym::i32_legacy_fn_max_value) => Some(Constant::Int(i32::MAX as u128)),
Some(sym::i64_legacy_fn_max_value) => Some(Constant::Int(i64::MAX as u128)),
Some(sym::i128_legacy_fn_max_value) => Some(Constant::Int(i128::MAX as u128)),
Some(sym::mem_align_of) => self.align_of_call(callee),
Some(sym::mem_size_of) => self.size_of_call(callee),
_ => None,
},
ExprKind::Call(callee, [..]) => match self.get_fn_diagnostic_name(callee) {
Some(sym::i8_legacy_fn_max_value) => Some(Constant::Int(i8::MAX as u128)),
Some(sym::i16_legacy_fn_max_value) => Some(Constant::Int(i16::MAX as u128)),
Some(sym::i32_legacy_fn_max_value) => Some(Constant::Int(i32::MAX as u128)),
Some(sym::i64_legacy_fn_max_value) => Some(Constant::Int(i64::MAX as u128)),
Some(sym::i128_legacy_fn_max_value) => Some(Constant::Int(i128::MAX as u128)),
Some(sym::mem_align_of) => self.align_of_call(callee),
Some(sym::mem_size_of) | Some(sym::mem_size_of_val)=> self.size_of_call(callee),
_ => None,
},

}
}

fn align_of_call(&self, callee: &Expr<'_>) -> Option<Constant<'tcx>> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some documentation here for these two functions?

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Sep 5, 2025
@blyxyas
Copy link
Member

blyxyas commented Sep 23, 2025

ping @RunDevelopment 🏓 I'm not sure if you saw my review or if it just flew under the radar?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants